-
Notifications
You must be signed in to change notification settings - Fork 14
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
warn against automatically using nightly features #23
Conversation
|
2dfc3ef
to
3d5dd47
Compare
@Nilstrieb A feature flag does not solve the same problem and hence is not a viable replacement. |
Using |
Perhaps a feature could have a version. If that could be accessed through |
@SergioBenitez any opinion on this? As-is this create implicitly recommends code to be written in a way that leads to nightly rustc development being harder than it should be, making it generally an anti-pattern (see rust-lang/rust#120804). |
I agree that we should include more verbiage with respect to how this crate can be used in a less error-prone fashion. On the broader subject, I believe there are two threads to consider:
The argument pushed forward by this PR and by the comment in tkaitchuck/aHash#200 (comment) is that the dependent should be responsible for opting-in to potential nightly breakage via a feature flag or similar. While I believe this is a fair recommendation, I also believe that this "opting-in" is already happening as a result of a nightly compiler being used, and that a second opting-in should be viewed as redundant. By their very nature, nightly compilers are expected to break. It shouldn't be seen as catastrophic when something does break. We shouldn't normalize using a nightly compiler or try to make it seem as stable as Finally, I believe the dependency should have the responsibility of staying on top of nightly releases, should they wish to use nightly features, and by doing so, decrease significantly or entirely eliminate any kind of breakage. This is the approach we took with Rocket early on and one we continue to take. It's a large responsibility, but it's attenuated by 1) the ability for users to roll-back to a "good" nightly release, and 2) the fact that not everyone updates their nightly toolchain immediately. In some ideal word, To summarize, I agree that we should add a word of warning to this crate's documentation, but my feeling is that that wording should be a bit more conservative than the proposed. |
As someone who uses nightly compilers a lot, I generally expect that So I must disagree here, using nightly is not an opt-in inviting rustc to just use potentially breaking features in whatever way it pleases. If that were true, we wouldn't bother with all these I also think historically this is pretty clear: the opt-in to a potential breakage is the Interpreting the use of nightly as opt-in is a risk for nightly rustc development since we can no longer easily test how a crate will behave when this compiler turns stable, and we can no longer change nightly features without risking making some crates that work fine on stable entirely impossible to build on nightly, thus severely impacting the usability of nightly compilers. If a crate wouldn't build on stable at all, then obviously none of this matters, but we're talking about crates that are ostensibly relying only on stable Rust, but then when I switch to nightly e.g. to use some |
I also use nightly almost exclusively and yet I don't have the same expectation. I generally expect that the compilers will build the same code, but I expect nearly 100% reliability from a stable compiler whereas I don't have the same expectation from a nightly compiler. Unfortunately it doesn't seem that any documentation anywhere defines what the user's expectations of each channel should be. I base my expectations on my own experience: ICEs and random compilation bugs happen with nightly with significant more frequency than
I think this is a good argument, and I think it would be an even stronger argument if It should also be mentioned that there are numerous differences in
The
I understand, but as we've covered, I feel that you're absolving the user from any responsibility involved with the switch to nightly. Their code could well stop building as a result of a change in the nightly compiler that has nothing to do with unstable features. In fact, there are dozens of issues reporting such breakage in the last couple of weeks. What is the analogous argument but to suggest that one shouldn't use the I should clarify that I am not against adding a word of caution to this crate's documentation, and I am in the process of doing so, only that it shouldn't bias so much in any one direction. |
Pushed a variant of this in f4284af. |
Yes, nightly has more compiler bugs. It exists as a testbed for the compiler. But it was never intended to be a testbed for ostensibly stable crates to silently enable more functionality. This is something crate authors started doing, and some individuals on the compiler team are not happy about that since it interferes with the original role of nightly. Incidents like the one with ahash make it noticeably harder to find and weed out these bugs in nightly before they hit beta and then stable. That's why we should push back hard against crates doing things like that. Some crates use a more sophisticated approach where they actually build a bit of demo code that uses the feature, and then only enable the feature if the demo code builds -- that has the big advantage that if the feature changes in ways that break the demo code, the crate keeps building. It is still problematic IMO, but it is much better than a crate just blindly enabling a nightly feature without any check whether that feature works the way it should. The latter is something that IMO should never be done.
I think you misunderstood the work-around proposed there -- that proposal is intended to deal with code that uses functions like
The beta channel is great, but when working on compiler features, if it takes 6 weeks until I can even test whether my code breaks anything that works on stable, that's really bad.
Crates that use I don't understand what kind of analogy you are trying to make here. It should be the case that if a crate builds on stable and doesn't build on nightly, that's a compiler bug (or an expected breakage of a stable feature, which is rare but happens). Bugs happen, obviously. They are not the user's responsibility. But crates that use Just look at the amount of backlinks in tkaitchuck/aHash#200. If that amount of fallout can arise from a nightly-only change not observable on stable, then something is wrong. This is not healthy. And the root of the problem is crates automatically enabling nightly features without even testing whether they work. That should be considered an anti-pattern and not acceptable to occur in any crate. Whether build probes are acceptable is still somewhat open; they are fragile in their own right. Thank you for adding some cautionary wording. I worry it might not be strong enough to dissuade people from using these functions. I can't think of any situation where I would consider the use of these functions legitimate (maybe a "this was built with nightly rustc" note in an auto-generated bugreport or so, that seems reasonable). My PR was the compromise version; what IMO really should happen is that these functions should be removed. Given that the crate provides no build probe functionality, it also seems unlikely people will switch to that instead. Maybe it'd be worth linking to https://github.com/cuviper/autocfg which provides a way to fairly easily write such build probes? |
See tkaitchuck/aHash#200 for an issue caused by using
version_check::supports_feature
without considering the consequences.IMO
supports_feature
should be removed entirely, but maybe there are ways to use it that are not as fragile?